FIX: SQLDescribeParam ordinal remapping for VARBINARY/BINARY NULL by jahnvi480 · Pull Request #654 · microsoft/mssql-python

jahnvi480 · 2026-06-30T09:31:02Z

Work Item / Issue Reference

AB#45521

GitHub Issue: #627

Summary

This pull request significantly improves how the driver resolves SQL types for NULL parameters, especially when inserting into temp tables or table variables. It proactively resolves unknown NULL parameter types before binding, emits actionable Python warnings when automatic type resolution fails, and provides clear guidance on using setinputsizes to avoid SQL Server implicit conversion errors. Additionally, the code is refactored for clarity and efficiency, and minor formatting improvements are made.

Improvements to NULL parameter type resolution:

Added a new PreResolveUnknownNullTypes function to proactively resolve unknown NULL SQL types for both single and batch execution paths, ensuring all types are determined before any parameters are bound. This prevents driver errors with binary NULLs and improves reliability.
When SQL type resolution falls back to SQL_VARCHAR (because SQLDescribeParam fails), the code now emits a Python warning with detailed guidance on how to use cursor.setinputsizes() to specify the correct type, preventing common SQL Server errors with BINARY/VARBINARY columns.

Documentation and user guidance:

Expanded the setinputsizes docstring in cursor.py to document the temp table/NULL issue and provide a concrete usage example for binary columns.

Code structure and clarity:

Refactored parameter binding code to avoid redundant type resolution during binding, since types are now resolved in advance.
Added a bool isFallback field to DescribedParamInfo to track when a fallback type is used.

Minor improvements:

Made minor formatting and whitespace changes for improved readability.

) Root cause: When SQLDescribeParam is called AFTER some parameters are already bound via SQLBindParameter, the ODBC Driver Manager remaps parameter ordinals, causing SQLSTATE 07009 errors or returning wrong types for VARBINARY/BINARY columns. Fix: Pre-resolve ALL unknown NULL parameter types via SQLDescribeParam BEFORE any SQLBindParameter call, using the existing per-handle cache. This ensures stable ordinals since no parameters are bound at describe time. Changes: - ddbc_bindings.cpp: Add PreResolveUnknownNullTypes() that scans paramInfos for SQL_C_DEFAULT + SQL_UNKNOWN_TYPE and resolves them via cache before binding. Called from both BindParameters (execute) and BindParameterArray (executemany) paths. - ddbc_bindings.cpp: Release GIL during SQLDescribeParam (network call to sp_describe_undeclared_parameters). - ddbc_bindings.cpp: Cache both successful and fallback results to avoid repeated network calls on statement reuse. Emit UserWarning with setinputsizes guidance when SQLDescribeParam fails. - ddbc_bindings.h: Add isFallback field to DescribedParamInfo to distinguish successful describes from fallback values. - cursor.py: Document setinputsizes workaround for temp table BINARY columns in docstring. - tests: Add 7 regression tests covering NULL VARBINARY/BINARY with non-NULL params, executemany, statement reuse, temp table warning, and setinputsizes workaround. Performance: Zero overhead for common case (no NULL params) due to early-exit scan. Cached path adds <1us. Fallback caching gives 2.5x speedup on repeated temp table queries vs prior approach. Closes #627

Copilot

Pull request overview

This PR addresses GH-627 by preventing SQLDescribeParam ordinal remapping failures when NULL parameters (notably BINARY/VARBINARY) are described after earlier parameters have already been bound. It does this by pre-resolving unknown NULL parameter SQL types before any binding occurs, adds an end-user warning when SQLDescribeParam fails and the driver must fall back to SQL_VARCHAR, and adds regression coverage and documentation.

Changes:

Add a pre-scan (PreResolveUnknownNullTypes) that resolves unknown NULL parameter SQL types ahead of binding for both execute() and executemany() paths.
Emit a Python UserWarning when SQLDescribeParam fails (fallback to SQL_VARCHAR) with guidance to use setinputsizes.
Add GH-627 regression tests and expand setinputsizes documentation with guidance for temp table/table variable NULL binary cases.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

File	Description
`mssql_python/pybind/ddbc_bindings.cpp`	Pre-resolves unknown NULL types before binding; adds `SQLDescribeParam` fallback warning and refactors binding to avoid interleaved describe+bind.
`mssql_python/pybind/ddbc_bindings.h`	Extends `DescribedParamInfo` with `isFallback` metadata.
`mssql_python/cursor.py`	Updates `setinputsizes` docstring with GH-627 guidance and example usage.
`tests/test_004_cursor.py`	Adds regression tests covering execute/executemany ordering, reuse path, and temp-table warning/workaround behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Warning: clarify 0-based index, show example with all params typed - Docstring: remove incorrect None placeholder claim, use ConstantsDDBC - Tests: drop temp tables after use, use named constants instead of magic numbers

github-actions · 2026-06-30T12:20:41Z

📊 Code Coverage Report

🔥 Diff Coverage 90%	🎯 Overall Coverage 80%	📈 Total Lines Covered: `6765` out of `8358` 📁 Project: `mssql-python`

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

mssql_python/pybind/ddbc_bindings.cpp (90.2%): Missing lines 546-547,563,585-586,764

Summary

Total: 61 lines
Missing: 6 lines
Coverage: 90%

mssql_python/pybind/ddbc_bindings.cpp

Lines 542-551

  542                 .attr("warn")(
  543                     "SQLDescribeParam failed for parameter index " + std::to_string(paramIndex) +
  544                         " (0-based). Falling back to SQL_VARCHAR for NULL binding. "
  545                         "This may cause 'Implicit conversion from data type varchar to varbinary "
! 546                         "is not allowed' errors for BINARY/VARBINARY columns. "
! 547                         "To fix: from mssql_python.constants import ConstantsDDBC; then call "
  548                         "cursor.setinputsizes() with explicit type info for every parameter. "
  549                         "Example (2 params): cursor.setinputsizes("
  550                         "[(ConstantsDDBC.SQL_INTEGER.value, 10, 0), "
  551                         "(ConstantsDDBC.SQL_VARBINARY.value, column_size, 0)])",

Lines 559-567

  559 }
  560 
  561 // GH-627: Resolve unknown NULL SQL types before any SQLBindParameter calls.
  562 // Some drivers remap parameter ordinals during describe when parameters have
! 563 // already been bound, so interleaving describe+bind can fail for binary NULLs.
  564 // When `params` is provided (execute path), an additional py::none check is
  565 // performed; for executemany (array path), SQL_C_DEFAULT already guarantees
  566 // all values in that column are NULL, so no Python-level check is needed.
  567 static void PreResolveUnknownNullTypes(SqlHandle& handle, SQLHANDLE hStmt,

Lines 581-590

  581     }
  582     if (!hasUnknownNull)
  583         return;
  584 
! 585     for (size_t paramIndex = 0; paramIndex < paramInfos.size(); ++paramIndex) {
! 586         ParamInfo& paramInfo = paramInfos[paramIndex];
  587         if (paramInfo.paramCType != SQL_C_DEFAULT || paramInfo.paramSQLType != SQL_UNKNOWN_TYPE) {
  588             continue;
  589         }
  590         // For execute(), verify the actual value is None (mixed columns possible).

Lines 760-768

  760             case SQL_C_DEFAULT: {
  761                 if (!py::isinstance<py::none>(param)) {
  762                     ThrowStdException(MakeParamMismatchErrorStr(paramInfo.paramCType, paramIndex));
  763                 }
! 764                 dataPtr = nullptr;  // GH-627: type resolved by PreResolveUnknownNullTypes.
  765                 strLenOrIndPtr = AllocateParamBuffer<SQLLEN>(paramBuffers);
  766                 *strLenOrIndPtr = SQL_NULL_DATA;
  767                 bufferLength = 0;
  768                 break;

📋 Files Needing Attention

📉 Files with overall lowest coverage (click to expand)

mssql_python.pybind.logger_bridge.cpp: 59.2%
mssql_python.pybind.ddbc_bindings.h: 59.9%
mssql_python.pybind.logger_bridge.hpp: 70.8%
mssql_python.pybind.connection.connection.cpp: 76.2%
mssql_python.pybind.ddbc_bindings.cpp: 76.3%
mssql_python.row.py: 76.9%
mssql_python.__init__.py: 77.3%
mssql_python.ddbc_bindings.py: 79.6%
mssql_python.logging.py: 85.5%
mssql_python.connection.py: 85.6%

🔗 Quick Links

⚙️ Build Summary	📋 Coverage Details
View Azure DevOps Build	Browse Full Coverage Report

…verage, add mixed-rows test

…dge case tests

…owBatch_wrap

… jahnvi/fix-sqldescribeparam-varbinary-627 # Conflicts: # mssql_python/pybind/ddbc_bindings.cpp

Copilot AI review requested due to automatic review settings June 30, 2026 09:31

github-actions Bot added the pr-size: medium Moderate update size label Jun 30, 2026

Copilot started reviewing on behalf of jahnvi480 June 30, 2026 09:31 View session

Copilot AI reviewed Jun 30, 2026

View reviewed changes

Comment thread mssql_python/pybind/ddbc_bindings.cpp Outdated

Comment thread mssql_python/cursor.py Outdated

Comment thread tests/test_004_cursor.py

Comment thread tests/test_004_cursor.py

Comment thread tests/test_004_cursor.py Outdated

Address PR review: fix warning message, docstring, test cleanup

c243a97

- Warning: clarify 0-based index, show example with all params typed - Docstring: remove incorrect None placeholder claim, use ConstantsDDBC - Tests: drop temp tables after use, use named constants instead of magic numbers

Address review: remove isFallback, add empty() guard, improve diff co…

6f2f382

…verage, add mixed-rows test

jahnvi480 marked this pull request as draft June 30, 2026 13:02

jahnvi480 added 6 commits June 30, 2026 18:40

Address review round 3: cache-before-warn, try-catch, bounds check, e…

18ac27a

…dge case tests

Fix warning message, revert formatting, add CHANGELOG, fix linting

ffe9ade

Revert unintended formatting changes in SQLExecuteMany_wrap, FetchArr…

638ac78

…owBatch_wrap

Restore removed blank lines to match main

006ca6d

Merge branch 'main' of https://github.com/microsoft/mssql-python into…

9c28c7b

… jahnvi/fix-sqldescribeparam-varbinary-627 # Conflicts: # mssql_python/pybind/ddbc_bindings.cpp

Fix stray BindParameters line split to match main

690eed3

jahnvi480 marked this pull request as ready for review July 1, 2026 08:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

FIX: SQLDescribeParam ordinal remapping for VARBINARY/BINARY NULL#654

FIX: SQLDescribeParam ordinal remapping for VARBINARY/BINARY NULL#654
jahnvi480 wants to merge 9 commits into
mainfrom
jahnvi/fix-sqldescribeparam-varbinary-627

jahnvi480 commented Jun 30, 2026 •

edited by azure-boards Bot

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 30, 2026 •

edited

Loading

🔥 Diff Coverage

90%

🎯 Overall Coverage

80%

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

jahnvi480 commented Jun 30, 2026 • edited by azure-boards Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Work Item / Issue Reference

Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Jun 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Code Coverage Report

🔥 Diff Coverage

90%

🎯 Overall Coverage

80%

Diff Coverage

Diff: main...HEAD, staged and unstaged changes

Summary

mssql_python/pybind/ddbc_bindings.cpp

📋 Files Needing Attention

🔗 Quick Links

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jahnvi480 commented Jun 30, 2026 •

edited by azure-boards Bot

Loading

github-actions Bot commented Jun 30, 2026 •

edited

Loading